add support for path references #90
Conversation
this is going to make my life suck significantly less need to fix * slurp up channel usages and flake inputs at index time and add as external library roots * probably worthwhile to embed nix binaries into the ide plugin
file if first class primitive, directory is not
nix repl isn't in the ij sandbox path, avoid using it and instead do the path handling in band this breaks handling of channel paths like <nixpkgs> but i don't care about those, would rather have something stable than something complete (also i only care about flakes)
| expr_std_path ::= PATH_SEGMENT (PATH_SEGMENT | antiquotation)* PATH_END | ||
| expr_std_path ::= PATH_SEGMENT (PATH_SEGMENT | antiquotation)* PATH_END { | ||
| mixin="org.nixos.idea.psi.impl.NixExprStdPathMixin" | ||
| } |
There was a problem hiding this comment.
question: Do you really need both, a reference contributor and the mixin? As far as I know, this are two different ways to implement references, you shouldn't need both.
There was a problem hiding this comment.
The mixin calls into the reference contributor here.
Seems like we don't need both but everyone uses the reference contributor + the mixin it seems. Without this explicit call the reference contributor will not work so I think you actually need both or can get away with only the mixin but not only the reference contributor.
src/main/java/org/nixos/idea/imports/NixFilePathReferenceContributor.kt
Outdated
Show resolved
Hide resolved
| import com.intellij.util.ProcessingContext | ||
| import org.nixos.idea.psi.impl.NixExprStdPathMixin | ||
|
|
||
| class NixFilePathReferenceContributor: PsiReferenceContributor() { |
There was a problem hiding this comment.
issue (non-blocking): Would be nice to have some test to cover this functionality. However, not sure how a test could look like. I have some tests regarding references in SymbolNavigationTest, but their setup is probably far too complex for your use case, and it also doesn't support asserting for references to files.
It is not a hard requirement before I merge the PR, but it would be nice.
There was a problem hiding this comment.
yeah i thought about this a bit and concluded idk how to write the tests. maybe we can pair on it together? i'm happy to schedule some time over the next few weeks if you're up for it.
There was a problem hiding this comment.
You mean as a live call? I somehow didn't expect that in the context of a pull request on GitHub. 😄 Unfortunately, it is difficult for me to plan specific times before the start of January. Not sure if I want to wait for January. I suppose you are not on the 38C3 in Hamburg (Germany), are you?^^
There was a problem hiding this comment.
yeah! a live call! i prefer it as we can communicate a bit faster. seconds rtt instead of minutes / hours / days.
no plans to go to this 38C3 thing, seems interesting though maybe that could change, will see whether it makes sense.
what if we do this, let's merge the PR if you're fine with the lack of tests, let's get a time on the books (probably 30m) for early Jan and if i make it to 38C3 (unlikely though) we can meetup and solve it in real life? how does jan 1 sound (probably like after 5pm germany time?)
This PR adds go to declaration for nix path references.
Screen.Recording.2024-12-07.at.8.00.34.PM.mov
I've kept it as simple as possible. It only supports stdpaths and not lookup paths. References are resolved following a simple set of rules.
nix-idea/src/main/java/org/nixos/idea/imports/NixFilePathReferenceContributor.kt
Lines 54 to 63 in 71a79a4
I spend hours a day reading nix files in IJ and this plugin is an important part of making this bearable. Thanks for making this, its really nice.